Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom elements manifest #31673

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

janechu
Copy link
Contributor

@janechu janechu commented Jun 11, 2024

Previous Behavior

No custom element manifest.

New Behavior

Added a custom element manifest.
Added VS Code HTML custom data.
Corrected some erroneous code comments.
Removed a few imports from @microsoft/fast-web-utilities as we cannot resolve the types (or at least I had trouble) and that package has been deprecated.

This will improve developer experience when using the web components package.

Reviewers

Please note the addition of a plugin to specify the tagName for a customElement, this happens at the post process step when creating a manifest. There doesn't seem to be another way to reverse-engineer getting the tagName unless @customElement decorator is used (I believe if we used that pattern we might get that for free). This assumes that the code comments are correct and uses TypeScript AST to check.

Also a plugin has been added to normalize TypeScript types into their values. Since we use const object a lot, it will use TypeScript to find these const objects and turn them into a format that an additional plugin uses to convert the CEM to HTML custom data.

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 11, 2024

@janechu
Copy link
Contributor Author

janechu commented Jun 18, 2024

Holding on this favor of #31689 which might need adjustments to account for tagName as added in this PR.

@janechu janechu force-pushed the users/janechu/add-cem branch from eb37b82 to 7d2ee97 Compare July 25, 2024 21:36
@fabricteam
Copy link
Collaborator

🕵 fluentui-web-components-v3 No visual regressions between this PR and main

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 39 47 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 652 625 5000
Button mount 313 321 5000
Field mount 1121 1176 5000
FluentProvider mount 710 744 5000
FluentProviderWithTheme mount 93 89 10
FluentProviderWithTheme virtual-rerender 39 47 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 87 82 10
MakeStyles mount 864 887 50000
Persona mount 1776 1730 5000
SpinButton mount 1412 1425 5000
SwatchPicker mount 1696 1675 5000

@fabricteam
Copy link
Collaborator

🕵 fluentuiv8 No visual regressions between this PR and main

@fabricteam
Copy link
Collaborator

🕵 FluentUIV0 No visual regressions between this PR and main

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 98 84 1.17:1
ChatDuplicateMessagesPerf.default 161 147 1.1:1
AttachmentMinimalPerf.default 90 83 1.08:1
FlexMinimalPerf.default 164 152 1.08:1
BoxMinimalPerf.default 203 190 1.07:1
CardMinimalPerf.default 315 295 1.07:1
LoaderMinimalPerf.default 204 190 1.07:1
LayoutMinimalPerf.default 207 196 1.06:1
TextAreaMinimalPerf.default 313 294 1.06:1
ChatWithPopoverPerf.default 202 192 1.05:1
HeaderSlotsPerf.default 489 466 1.05:1
RefMinimalPerf.default 107 102 1.05:1
SegmentMinimalPerf.default 202 193 1.05:1
ButtonSlotsPerf.default 317 306 1.04:1
GridMinimalPerf.default 203 195 1.04:1
PortalMinimalPerf.default 87 84 1.04:1
AnimationMinimalPerf.default 305 296 1.03:1
AvatarMinimalPerf.default 114 111 1.03:1
ButtonMinimalPerf.default 92 89 1.03:1
ChatMinimalPerf.default 438 425 1.03:1
DropdownManyItemsPerf.default 395 384 1.03:1
HeaderMinimalPerf.default 214 207 1.03:1
LabelMinimalPerf.default 224 217 1.03:1
ProviderMergeThemesPerf.default 645 629 1.03:1
SkeletonMinimalPerf.default 203 197 1.03:1
StatusMinimalPerf.default 400 387 1.03:1
DropdownMinimalPerf.default 1428 1406 1.02:1
EmbedMinimalPerf.default 1886 1853 1.02:1
ListMinimalPerf.default 305 298 1.02:1
RadioGroupMinimalPerf.default 269 264 1.02:1
ToolbarMinimalPerf.default 549 538 1.02:1
TreeMinimalPerf.default 480 469 1.02:1
AlertMinimalPerf.default 160 158 1.01:1
DatepickerMinimalPerf.default 3543 3525 1.01:1
ItemLayoutMinimalPerf.default 702 696 1.01:1
RosterPerf.default 1600 1584 1.01:1
SliderMinimalPerf.default 747 739 1.01:1
SplitButtonMinimalPerf.default 2298 2279 1.01:1
IconMinimalPerf.default 400 397 1.01:1
TableManyItemsPerf.default 1101 1090 1.01:1
CustomToolbarPrototype.default 1477 1466 1.01:1
CarouselMinimalPerf.default 256 256 1:1
CheckboxMinimalPerf.default 1134 1139 1:1
FormMinimalPerf.default 224 225 1:1
ReactionMinimalPerf.default 216 216 1:1
TextMinimalPerf.default 198 198 1:1
TooltipMinimalPerf.default 1310 1310 1:1
VideoMinimalPerf.default 443 442 1:1
AttachmentSlotsPerf.default 626 632 0.99:1
ButtonOverridesMissPerf.default 667 673 0.99:1
DialogMinimalPerf.default 441 444 0.99:1
InputMinimalPerf.default 532 540 0.99:1
ListWith60ListItems.default 376 379 0.99:1
MenuMinimalPerf.default 503 508 0.99:1
DividerMinimalPerf.default 205 209 0.98:1
MenuButtonMinimalPerf.default 951 968 0.98:1
ProviderMinimalPerf.default 202 208 0.97:1
TableMinimalPerf.default 232 239 0.97:1
ImageMinimalPerf.default 222 231 0.96:1
PopupMinimalPerf.default 338 352 0.96:1
AccordionMinimalPerf.default 82 86 0.95:1
ListCommonPerf.default 394 418 0.94:1
ListNestedPerf.default 314 341 0.92:1

@@ -0,0 +1,7 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual regressions to review in the fluentuiv9 Visual Regression Report

Avatar Converged 2 screenshots
Image Name Diff(in Pixels) Image Type
Avatar Converged.badgeMask.normal.chromium.png 2 Changed
Avatar Converged.badgeMask - RTL.normal.chromium.png 1 Changed

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 617 628 5000
Breadcrumb mount 1679 1681 1000
Checkbox mount 1667 1706 5000
CheckboxBase mount 1456 1469 5000
ChoiceGroup mount 2898 2966 5000
ComboBox mount 674 668 1000
CommandBar mount 6514 6572 1000
ContextualMenu mount 13226 13400 1000
DefaultButton mount 796 804 5000
DetailsRow mount 2211 2225 5000
DetailsRowFast mount 2259 2176 5000
DetailsRowNoStyles mount 2030 2043 5000
Dialog mount 2675 2861 1000
DocumentCardTitle mount 230 240 1000
Dropdown mount 1974 1994 5000
FocusTrapZone mount 1132 1123 5000
FocusZone mount 1117 1065 5000
GroupedList mount 42520 42166 2
GroupedList virtual-rerender 20381 20416 2
GroupedList virtual-rerender-with-unmount 51855 51639 2
GroupedListV2 mount 240 227 2
GroupedListV2 virtual-rerender 216 220 2
GroupedListV2 virtual-rerender-with-unmount 229 227 2
IconButton mount 1114 1128 5000
Label mount 350 344 5000
Layer mount 2765 2770 5000
Link mount 404 399 5000
MenuButton mount 977 986 5000
MessageBar mount 21393 21449 5000
Nav mount 2024 2077 1000
OverflowSet mount 786 757 5000
Panel mount 1815 1875 1000
Persona mount 739 749 1000
Pivot mount 904 909 1000
PrimaryButton mount 936 910 5000
Rating mount 4684 4739 5000
SearchBox mount 922 919 5000
Shimmer mount 1900 1889 5000
Slider mount 1313 1349 5000
SpinButton mount 3005 3000 5000
Spinner mount 397 406 5000
SplitButton mount 1840 1912 5000
Stack mount 425 448 5000
StackWithIntrinsicChildren mount 880 889 5000
StackWithTextChildren mount 2734 2756 5000
SwatchColorPicker mount 6328 6365 5000
TagPicker mount 1461 1451 5000
Text mount 385 386 5000
TextField mount 927 923 5000
ThemeProvider mount 866 860 5000
ThemeProvider virtual-rerender 578 598 5000
ThemeProvider virtual-rerender-with-unmount 1298 1308 5000
Toggle mount 625 608 5000
buttonNative mount 191 200 5000

@janechu janechu force-pushed the users/janechu/add-cem branch from ab3ff2d to ed1b59b Compare July 27, 2024 05:33
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get dirname from import.meta.dirname.

})
}
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: to improve readability, maybe test negative cases and return? Something like:

if (!Array.isArray(...)) {
  return;
}
sourceFile.statements.forEach(...);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @marchbox's suggestion these nested ifs could be easier to read. Can sourceFile.statements, statement.jsDoc, or jsDoc.tags not be arrays?

Copy link
Contributor

@davatron5000 davatron5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some questions. + @marchbox's feedback.

* This plugin adds the tagName after the manifest has been processed
* See: https://github.com/webcomponents/custom-elements-manifest/blob/main/schema.json
*/
export function tagNameFix() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I understand, we need this plugin because we dynamically prefix our component names? And then this goes thru and yoinks the name from the comment. What is the tagName set to before this runs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants